Skip to content

BUG: Series.nlargest thinks True < False #26157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 23, 2019
Merged

BUG: Series.nlargest thinks True < False #26157

merged 1 commit into from
Apr 23, 2019

Conversation

swyoon
Copy link
Contributor

@swyoon swyoon commented Apr 20, 2019 •
edited
Loading

@codecov
Copy link

codecov bot commented Apr 20, 2019

Codecov Report

Merging #26157 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26157      +/-   ##
==========================================
- Coverage      92%   91.99%   -0.01%     
==========================================
  Files         175      175              
  Lines       52371    52373       +2     
==========================================
- Hits        48184    48182       -2     
- Misses       4187     4191       +4
Flag Coverage Δ
#multiple 90.55% <100%> (ø) ⬆️
#single 40.74% <0%> (-0.15%) ⬇️
Impacted Files Coverage Δ
pandas/core/algorithms.py 94.8% <100%> (+0.01%) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.9% <0%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c79b7bb...f121e7e. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 20, 2019 •
edited
Loading

Codecov Report

Merging #26157 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26157      +/-   ##
==========================================
- Coverage   91.98%   91.98%   -0.01%     
==========================================
  Files         175      175              
  Lines       52371    52373       +2     
==========================================
- Hits        48175    48174       -1     
- Misses       4196     4199       +3
Flag Coverage Δ
#multiple 90.53% <100%> (ø) ⬆️
#single 40.7% <0%> (-0.16%) ⬇️
Impacted Files Coverage Δ
pandas/core/algorithms.py 94.8% <100%> (+0.01%) ⬆️
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 96.9% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.71% <0%> (+0.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 947bd76...3259878. Read the comment docs.

@swyoon swyoon force-pushed the gh26154 branch 2 times, most recently from 7cb4f28 to 5708b24 Compare April 20, 2019 05:30
@swyoon
Copy link
Contributor Author

swyoon commented Apr 20, 2019

@WillAyd I don't think the failed test case is related to this PR. Would you please take a look?

@gfyoung gfyoung added Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff Bug labels Apr 20, 2019
@swyoon
Copy link
Contributor Author

swyoon commented Apr 20, 2019

@gfyoung PR is updated as asked. Could you please take a look? The broken CI seems not relevant to this PR.

@gfyoung
Copy link
Member

gfyoung commented Apr 21, 2019

@swyoon : That's very odd...can you try rebasing / merging master to see if that fixes it?

@swyoon
Copy link
Contributor Author

swyoon commented Apr 21, 2019

@gfyoung the CI failure has been fixed. all checks are green.

@jreback jreback changed the title BUG: Sereis.nlargest thinks True < False BUG: Series.nlargest thinks True < False Apr 21, 2019
jreback
jreback requested changes Apr 21, 2019
View reviewed changes
@swyoon
BUG: Sereis.nlargest thinks True < False
3259878 
- use 1-arr insteat of -arr for sorting a boolean array casted to uint
- add test
- add whatsnew
@swyoon
Copy link
Contributor Author

swyoon commented Apr 22, 2019

@jreback the tests are green.

@swyoon
Copy link
Contributor Author

swyoon commented Apr 23, 2019

@jreback Could you please take a look?

@jreback jreback added this to the 0.25.0 milestone Apr 23, 2019
@jreback jreback merged commit a13ef66 into pandas-dev:master Apr 23, 2019
@jreback
Copy link
Contributor

jreback commented Apr 23, 2019

thanks @swyoon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

@gfyoung gfyoung gfyoung approved these changes

Assignees
No one assigned
Labels
Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff Bug
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.

Nlargest on boolean return False first
3 participants
@swyoon @gfyoung @jreback